Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move syntax.md to build-tools, improve semconv README. #1825

Merged
merged 3 commits into from
Jul 26, 2021

Conversation

Oberon00
Copy link
Member

Follow-up to open-telemetry/build-tools#53: We don't need the syntax.md in two different places, if we already have a README with a link, just point the link to build-tools.

Also add a helpful warning that those who want to read semantic conventions confusingly should not look into /semantic_conventions/ but /specification/<signal>/semantic_conventions/.

No CHANGELOG entry, as this is only a editorial change.

@Oberon00 Oberon00 added area:semantic-conventions Related to semantic conventions spec:metrics Related to the specification/metrics directory spec:resource Related to the specification/resource directory spec:trace Related to the specification/trace directory editorial Editorial changes only (typos, changelog, ...). No content-related changes of any kind. labels Jul 22, 2021
@Oberon00 Oberon00 requested review from a team July 22, 2021 15:46
@yurishkuro
Copy link
Member

yurishkuro commented Jul 22, 2021

Not sure I agree with the move of the syntax.md file. It's a documentation file about how to write specs, which applies to files that exist in the specs, so why should be in a different repo? How is it even related to "tools", which are an implementation detail of the spec build process?

@Oberon00
Copy link
Member Author

How is it even related to "tools", which are an implementation detail of the spec build process?

The syntax.md file documents the input format of the tool that generates the markdown output of the spec. IMHO the whole semantic_conventions/ top-level directory is as much an implementation detail of the spec as the semantic conventions generator tool.

The reason for the move was described in open-telemetry/build-tools#53, and it's a very practical one: As soon as you change the input format of the tool, you need to update syntax.md. If you have both in the same repository, you can update them in one PR and the changes to syntax.md also serve as documentation to the changes in the implementing Python code.

@Oberon00
Copy link
Member Author

Oberon00 commented Jul 22, 2021

Since we have the (now required) semantic-conventions check as part of every PR, you can't even write anything in the YAML that the tool doesn't support, even if it would maybe make sense conceptually, so the tool is a quite important "implementation detail" 😃

Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair enough

@Oberon00
Copy link
Member Author

Heads-up: I created a PR to update syntax.md in the build tools, since it was not up to date with some recent additions to the tool. I also documented features that were there from the beginning but are not currently used in the spec (with a warning): open-telemetry/build-tools#56

@carlosalberto carlosalberto merged commit 3422558 into open-telemetry:main Jul 26, 2021
@Oberon00 Oberon00 deleted the move-syntax branch July 26, 2021 14:42
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:semantic-conventions Related to semantic conventions editorial Editorial changes only (typos, changelog, ...). No content-related changes of any kind. spec:metrics Related to the specification/metrics directory spec:resource Related to the specification/resource directory spec:trace Related to the specification/trace directory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants